Skip to content

fix(storage): avoid stomping over session-level auto_decompress value#718

Open
TheKevJames wants to merge 1 commit intokjames/fix-auth-aiohttp-compatfrom
kjames/fix-storage-auto-decompress-override
Open

fix(storage): avoid stomping over session-level auto_decompress value#718
TheKevJames wants to merge 1 commit intokjames/fix-auth-aiohttp-compatfrom
kjames/fix-storage-auto-decompress-override

Conversation

@TheKevJames
Copy link
Member

Summary

Blocked on #717

@TheKevJames TheKevJames requested a review from a team as a code owner April 6, 2024 12:46
@TheKevJames TheKevJames requested review from cphoward, eddiedialpad and juanamari94 and removed request for a team April 6, 2024 12:46
# is present, then the client will handle the decompression
auto_decompress = 'accept-encoding' not in {k.lower() for k in headers}
auto_decompress = None # inherit the Session default setting
if 'accept-encoding' not in {k.lower() for k in headers}:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if 'accept-encoding' not in {k.lower() for k in headers}:
if 'accept-encoding' in {k.lower() for k in headers}:

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we want want Kevin has here. If 'accept-encoding' in {k.lower() for k in headers} is True, that means that the session passes auto_decompress value implicitly in session, and we want None. We only want to set auto_decompress if 'accept-encoding' in {k.lower() for k in headers} is False, that is 'accept-encoding' is not in the headers.

Copy link
Contributor

@eddiedialpad eddiedialpad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

approved: avoid overriding auto_decompress value

# is present, then the client will handle the decompression
auto_decompress = 'accept-encoding' not in {k.lower() for k in headers}
auto_decompress = None # inherit the Session default setting
if 'accept-encoding' not in {k.lower() for k in headers}:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we want want Kevin has here. If 'accept-encoding' in {k.lower() for k in headers} is True, that means that the session passes auto_decompress value implicitly in session, and we want None. We only want to set auto_decompress if 'accept-encoding' in {k.lower() for k in headers} is False, that is 'accept-encoding' is not in the headers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants